Make better use of i18n #20096

Gusted · 2022-06-23T03:28:12Z

Previously, we relied on performing numerous expensive map lookups and storing each parsed ini file in memory.
As we add more translations and languages, this strategy won't scale and has become visible and measurable in memory utilization. All parsed ini currently uses about 20-30Mb of memory, which is nothing on modern computers. Every bit counts when running Gitea on low-powered devices like Raspberry Pis and Android phones. This PR fixes that by using a new data structure that don't require more memory than is necessary and is scalable for the future.
Startup memory is now closer to 60Mb than 90Mb, making it easier to stay under 100Mb. The new implementation needs 7Mb to hold a single map for translation key -> index and for each locale a maps index -> translation value.
The lookup for translation values may be greatly improved, leading to faster performance overall, even though I haven't performed any tests or benchmarks comparing the current and earlier implementations. Additionally, the language struct, which contains the important map for index -> translation value, so you don't have to do another map lookup to get the locale struct.

Before:

25Mb to initialize the locales(After Go's GC)

After:

7Mb to initialize the locales(After Go's GC)

As usual, goodluck reviewing this piece of beauty and mess.

wxiaoguang · 2022-06-23T04:22:57Z

I think it's on the correct direction for the i18n refactoring.

The only question in my mind is: is the perfect hash map necessary? If the simple map can still save the memory and works fine, I would prefer to use a simple map (like map[string][]string => map[key][offset]) to reduce maintenance work.

Gusted · 2022-06-24T01:27:30Z

is the perfect hash map necessary? If the simple map can still save the memory and works fine, I would prefer to use a simple map (like map[string][]string => map[key][offset]) to reduce maintenance work.

So, I'd preferred the perfect hash as it was made for these kind of situations, need fast query and the keys are known before-hand. Thereby there was no really good quality perfect hash library(so that's why I crafted one and included in the PR). I've pushed a commit(d498472) that utilized the same concept of the language offset and then instead use golang's Map feature. The memory is bit more on the heap(it says 10Mb) but the overall memory usage still seems to be around the same levels of what the previous implementation had.

6bfa1ba actually is the one that moves back the original data structure that is optimized for memory.

- Uses less memory by creating for each language a map.

wxiaoguang · 2022-06-24T01:56:00Z

L-G-T-M (except CI failure is related)

And, could the translationKeys be set to nil to save some memory? It seems that it's not used after the loop IIRC.

delvh · 2022-06-24T07:00:17Z

--- FAIL: TestLDAPUserSigninFailed (0.30s)
testlogger.go:78: 2022/06/24 01:51:14 modules/git/git.go:133:checkInit() [W] git module has been initialized already, duplicate init should be fixed

That issue once again...

wxiaoguang · 2022-06-24T07:03:37Z

--- FAIL: TestLDAPUserSigninFailed (0.30s)
testlogger.go:78: 2022/06/24 01:51:14 modules/git/git.go:133:checkInit() [W] git module has been initialized already, duplicate init should be fixed

That issue once again...

~~Off-topic~~ Unrelated .... just a warning, no harm. It might be resolved after another big git module refactoring.

delvh

I think, I got the gist of what you're doing.
If it works, LGTM.

modules/translation/i18n/i18n.go

modules/translation/translation.go

- Separate dev and production locale stores. - Allow for live-reloading in dev mode. Co-authored-by: zeripath <art27@cantab.net>

Gusted · 2022-06-25T21:25:32Z

Pushed commits to separate dev and production localestores(so it's easier to implement live-reloading etc.). Provided by @zeripath

wxiaoguang · 2022-06-25T23:54:00Z

I think some code can be simplified, will try on it later.

Gusted · 2022-06-25T23:55:00Z

I think some code can be simplified, will try on it later.

Even more refactors 😄 I will mark this PR for 1.19 already

wxiaoguang · 2022-06-25T23:57:01Z

I believe 1.18 is fine (will try on it later means several hours later, I am going outside soon 😂) .

wxiaoguang · 2022-06-26T10:49:38Z

I proposed a PR about my idea. Gusted#5

Major benefits:

more simple (+185 −509)
no more go-routine, no need to close (in case it's forgotten ...)
no duplicate code, only one Tr / reloadLocaleByIni function, more easier to be maintained
works fine for dev mode, the reload delay is (at most) 1 second, it won't affect development in real life: change locale files -> refresh page to see the result -> more than 1 second has been elapsed.

wxiaoguang

(approve, although I also made some changes, open for suggestions)

* giteaofficial/main: Add spacing between the properties of the key (go-gitea#20145) Remove U2F support (go-gitea#20141) Make better use of i18n (go-gitea#20096)

- Currently we're using the `i18n` variable naming for the `locale` struct. This contains locale's specific information and cannot be used for general i18n purpose, therefore refactoring it to `locale` makes more sense. - Ref: go-gitea#20096 (comment)

* Refactor `i18n` to `locale` - Currently we're using the `i18n` variable naming for the `locale` struct. This contains locale's specific information and cannot be used for general i18n purpose, therefore refactoring it to `locale` makes more sense. - Ref: #20096 (comment) * Update routers/install/install.go

* Prototyping * Start work on creating offsets * Modify tests * Start prototyping with actual MPH * Twiddle around * Twiddle around comments * Convert templates * Fix external languages * Fix latest translation * Fix some test * Tidy up code * Use simple map * go mod tidy * Move back to data structure - Uses less memory by creating for each language a map. * Apply suggestions from code review Co-authored-by: delvh <dev.lh@web.de> * Add some comments * Fix tests * Try to fix tests * Use en-US as defacto fallback * Use correct slices * refactor (go-gitea#4) * Remove TryTr, add log for missing translation key * Refactor i18n - Separate dev and production locale stores. - Allow for live-reloading in dev mode. Co-authored-by: zeripath <art27@cantab.net> * Fix live-reloading & check for errors * Make linter happy * live-reload with periodic check (go-gitea#5) * Fix tests Co-authored-by: delvh <dev.lh@web.de> Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: zeripath <art27@cantab.net>

* Refactor `i18n` to `locale` - Currently we're using the `i18n` variable naming for the `locale` struct. This contains locale's specific information and cannot be used for general i18n purpose, therefore refactoring it to `locale` makes more sense. - Ref: go-gitea#20096 (comment) * Update routers/install/install.go

Gusted added 12 commits June 21, 2022 03:12

Prototyping

24abae5

Start work on creating offsets

48b9b21

Modify tests

8b7ca1c

Start prototyping with actual MPH

8427dbd

Twiddle around

12605ec

Twiddle around comments

b25d0d5

Convert templates

f70b31a

Fix external languages

f466981

Fix latest translation

ddaf0cb

Fix some test

78bdc57

Merge branch 'main' into wow-maths

ace2fd1

Tidy up code

24c973b

Gusted added this to the 1.18.0 milestone Jun 23, 2022

Gusted added performance/memory Performance issues affecting memory use performance/cpu type/enhancement An improvement of existing functionality labels Jun 23, 2022

Merge branch 'main' into wow-maths

4788be5

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 23, 2022

Use simple map

d498472

Gusted added 3 commits June 24, 2022 03:27

Merge branch 'main' into wow-maths

c78f1f1

go mod tidy

957207b

Move back to data structure

6bfa1ba

- Uses less memory by creating for each language a map.

delvh approved these changes Jun 24, 2022

View reviewed changes

modules/translation/i18n/i18n.go Outdated Show resolved Hide resolved

modules/translation/i18n/i18n.go Outdated Show resolved Hide resolved

modules/translation/i18n/i18n.go Outdated Show resolved Hide resolved

modules/translation/translation.go Outdated Show resolved Hide resolved

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 24, 2022

zeripath and others added 4 commits June 25, 2022 20:00

Merge branch 'main' into wow-maths

2cffac5

Refactor i18n

8334002

- Separate dev and production locale stores. - Allow for live-reloading in dev mode. Co-authored-by: zeripath <art27@cantab.net>

Fix live-reloading & check for errors

f20fd87

Make linter happy

1978b74

Gusted requested review from delvh and wxiaoguang June 25, 2022 21:25

Gusted removed the performance/cpu label Jun 25, 2022

Merge branch 'main' into wow-maths

b252dfd

wxiaoguang and others added 3 commits June 26, 2022 11:05

live-reload with periodic check (#5)

05d490d

Fix tests

fbd3e08

Merge branch 'main' into wow-maths

f66ad02

Gusted added type/refactoring Existing code has been cleaned up. There should be no new functionality. and removed type/enhancement An improvement of existing functionality labels Jun 26, 2022

wxiaoguang approved these changes Jun 26, 2022

View reviewed changes

lunny approved these changes Jun 26, 2022

View reviewed changes

lunny merged commit 5d3f99c into go-gitea:main Jun 26, 2022

Gusted deleted the wow-maths branch June 26, 2022 14:22

Gusted mentioned this pull request Jun 27, 2022

Refactor i18n to locale #20153

Merged

wxiaoguang mentioned this pull request Jun 28, 2022

Make i18n live-reload work for default language and write-lock-free if no change #20165

Closed

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make better use of i18n #20096

Make better use of i18n #20096

Gusted commented Jun 23, 2022 •

edited

Loading

wxiaoguang commented Jun 23, 2022

Gusted commented Jun 24, 2022 •

edited

Loading

wxiaoguang commented Jun 24, 2022

delvh commented Jun 24, 2022

wxiaoguang commented Jun 24, 2022 •

edited

Loading

delvh left a comment

Gusted commented Jun 25, 2022

wxiaoguang commented Jun 25, 2022

Gusted commented Jun 25, 2022

wxiaoguang commented Jun 25, 2022

wxiaoguang commented Jun 26, 2022

wxiaoguang left a comment

Make better use of i18n #20096

Make better use of i18n #20096

Conversation

Gusted commented Jun 23, 2022 • edited Loading

Before:

After:

wxiaoguang commented Jun 23, 2022

Gusted commented Jun 24, 2022 • edited Loading

wxiaoguang commented Jun 24, 2022

delvh commented Jun 24, 2022

wxiaoguang commented Jun 24, 2022 • edited Loading

delvh left a comment

Choose a reason for hiding this comment

Gusted commented Jun 25, 2022

wxiaoguang commented Jun 25, 2022

Gusted commented Jun 25, 2022

wxiaoguang commented Jun 25, 2022

wxiaoguang commented Jun 26, 2022

wxiaoguang left a comment

Choose a reason for hiding this comment

Gusted commented Jun 23, 2022 •

edited

Loading

Gusted commented Jun 24, 2022 •

edited

Loading

wxiaoguang commented Jun 24, 2022 •

edited

Loading